-
Notifications
You must be signed in to change notification settings - Fork 333
http-client-java, bug fix, array encoding support closed enum #9413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
http-client-java, bug fix, array encoding support closed enum #9413
Conversation
|
No changes needing a change description found. |
|
You can try these changes here
|
|
could you please show the generated code diff by adding the fix? |
I cannot, because without this fix, emitter crashes, and there is no code generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for array encoding (comma-separated string representation) of closed enum types in the Java HTTP client generator. The change introduces a new ConvertFromJsonTypeTrait interface and updates the deserialization logic to properly handle enum values when deserializing from array-encoded strings.
Changes:
- Introduced
ConvertFromJsonTypeTraitinterface for converting JSON wire values back to client types - Implemented the new trait on
PrimitiveType,ClassType, andEnumTypeto support bidirectional conversion - Fixed array encoding deserialization logic in
StreamSerializationModelTemplateto use the new conversion trait instead of relying ondefaultValueExpression - Added unit tests for both
ConvertToJsonTypeTraitandConvertFromJsonTypeTraitconversion methods
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ConvertFromJsonTypeTrait.java | New interface defining conversion from JSON wire type to client type |
| ConvertToJsonTypeTrait.java | Updated documentation to clarify it converts to JSON wire type |
| PrimitiveType.java | Implements ConvertFromJsonTypeTrait and updates convertToJsonType to properly handle client-to-wire conversion |
| ClassType.java | Implements ConvertFromJsonTypeTrait with explicit handling for various special types like DATE_TIME, DURATION, URL |
| EnumType.java | Implements ConvertFromJsonTypeTrait to support enum deserialization from JSON values |
| StreamSerializationModelTemplate.java | Fixes array encoding deserialization to use ConvertFromJsonTypeTrait instead of defaultValueExpression, with proper error handling |
| ConvertToJsonTypeTraitTests.java | New test file verifying type conversion to JSON representation |
| ConvertFromJsonTypeTraitTests.java | New test file verifying type conversion from JSON representation |
...osoft/typespec/http/client/generator/core/model/clientmodel/ConvertToJsonTypeTraitTests.java
Show resolved
Hide resolved
...oft/typespec/http/client/generator/core/model/clientmodel/ConvertFromJsonTypeTraitTests.java
Show resolved
Hide resolved
...main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/ClassType.java
Outdated
Show resolved
Hide resolved
…/src/main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/ClassType.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
...main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/ClassType.java
Show resolved
Hide resolved
.../java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/PrimitiveType.java
Show resolved
Hide resolved
...main/java/com/microsoft/typespec/http/client/generator/core/model/clientmodel/ClassType.java
Show resolved
Hide resolved
missed one `return` in PR #9413 commit d12f548 tested in Azure/autorest.java#3271 https://dev.azure.com/azure-sdk/public/_build/results?buildId=5777927&view=results
test not yet published https://github.com/microsoft/typespec/blob/main/packages/http-specs/specs/encode/array/main.tsp#L44-L62